cpp_test first commit #25

damcav64 · 2026-01-26T05:54:12Z

No description provided.

rdkcmf-jenkins · 2026-01-26T05:56:56Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

Protex Server Path: /home/blackduck/github/firebolt-cpp-client/25/rdkcentral/firebolt-native-transport
Commit: af19817

Report detail: gist'

Copilot

Pull request overview

This PR introduces a new C++ test/demo application (cpp_test) for the Firebolt SDK. The application provides an interactive demonstration framework for testing various Firebolt interfaces including Accessibility, Advertising, Device, Display, Lifecycle, Localization, Presentation, and Stats modules.

Changes:

Added a new test application structure with main entry point and demo framework
Implemented demo classes for eight Firebolt interface modules
Added utility classes for HTTP communication, event triggering, and output stream handling
Integrated build configuration with CMake

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
test/cpp_test/main.cpp	Main entry point with connection management, command-line argument parsing, and interface selection loop
test/cpp_test/cpp/utils.h	Utility function declarations for HTTP operations and event verification
test/cpp_test/cpp/utils.cpp	Implementation of HTTP helpers using libcurl and event triggering functions
test/cpp_test/cpp/outputstream.h	Output stream wrapper class for redirecting output to file or console
test/cpp_test/cpp/fireboltdemo.h	Base class interface for Firebolt demo implementations
test/cpp_test/cpp/fireboltdemo.cpp	Base class implementation with RPC method discovery and user interaction
test/cpp_test/cpp/chooseInterface.h	Interface selection and coordination class header
test/cpp_test/cpp/chooseInterface.cpp	Implementation of interface selection with auto-run capability
test/cpp_test/cpp/accessibilityDemo.{h,cpp}	Demo implementation for Accessibility module testing
test/cpp_test/cpp/advertisingDemo.{h,cpp}	Demo implementation for Advertising module testing
test/cpp_test/cpp/deviceDemo.{h,cpp}	Demo implementation for Device module testing
test/cpp_test/cpp/displayDemo.{h,cpp}	Demo implementation for Display module testing (missing copyright headers)
test/cpp_test/cpp/lifecycleDemo.{h,cpp}	Demo implementation for Lifecycle module with state management
test/cpp_test/cpp/localizationDemo.{h,cpp}	Demo implementation for Localization module testing
test/cpp_test/cpp/presentationDemo.{h,cpp}	Demo implementation for Presentation module with event triggering
test/cpp_test/cpp/statsDemo.{h,cpp}	Demo implementation for Stats module testing
test/cpp_test/cpp/firebolt-open-rpc.json.in	Template for embedding OpenRPC specification
test/cpp_test/CMakeLists.txt	Build configuration for cpp_test executable with dependencies
test/cpp_test/.vscode/launch.json	VSCode debug configuration with hard-coded paths
CMakeLists.txt	Added cpp_test subdirectory to demo app build

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-26T05:58:36Z

test/cpp_test/main.cpp

+
+    if (gAutoRun)
+    {
+        gOutput = OutputStream("firebolt_test_output.txt");


The OutputStream is being recreated with the same filename. This line appears redundant since gOutput was already initialized with this filename at line 57. If the intention is to reset or flush the stream, this should be clarified.

Suggested change

gOutput = OutputStream("firebolt_test_output.txt");

test/cpp_test/cpp/presentationDemo.cpp

test/cpp_test/cpp/outputstream.h

Copilot · 2026-01-26T05:58:37Z

test/cpp_test/cpp/displayDemo.h

+#include <string>
+#include <vector>
+#include "fireboltdemo.h"
+
+class DisplayDemo : public IFireboltDemo 
+{
+public:
+    DisplayDemo();
+    ~DisplayDemo() = default;
+    void runOption(const int index);
+};


This header file is missing the copyright and license header that is present in all other demo header files in this directory. All other demo headers (accessibilityDemo.h, advertisingDemo.h, deviceDemo.h, lifecycleDemo.h, localizationDemo.h, presentationDemo.h, statsDemo.h) include the Comcast copyright notice and Apache 2.0 license header.

test/cpp_test/main.cpp

test/cpp_test/.vscode/launch.json

Copilot · 2026-01-26T05:58:39Z

test/cpp_test/cpp/chooseInterface.cpp

+    interfaces = {new AccessibilityDemo(), new AdvertisingDemo(),  new DeviceDemo(),       new DisplayDemo(),
+                  new LifecycleDemo(),     new LocalizationDemo(), new PresentationDemo(), new StatsDemo()};


Raw pointers are allocated with new but could leak if an exception is thrown during construction of subsequent objects in the initializer list. Consider using std::unique_ptr or std::shared_ptr for automatic memory management. The destructor correctly deletes them, but exception-safe initialization would be more robust.

Copilot · 2026-01-26T05:58:39Z

CMakeLists.txt

+    add_subdirectory(test/cpp_test) #for now
 endif()

 if (ENABLE_TESTS)
    add_subdirectory(test)


The comment "for now" suggests this is a temporary change. If this test directory should be built as part of the demo app conditionally, the comment should clarify the future plan, or this should be added under ENABLE_TESTS instead of ENABLE_DEMO_APP.

Suggested change

add_subdirectory(test/cpp_test) #for now

endif()

if (ENABLE_TESTS)

add_subdirectory(test)

endif()

if (ENABLE_TESTS)

add_subdirectory(test)

add_subdirectory(test/cpp_test)

test/cpp_test/cpp/displayDemo.cpp

Copilot · 2026-01-26T05:58:40Z

test/cpp_test/main.cpp

+#include <chrono>
+#include <iostream>
+#include <string>
+#include <thread>
+#include <unistd.h>
+
+#include "cpp/chooseInterface.h"
+
+#include "cpp/outputstream.h"
+
+OutputStream gOutput;


This main.cpp file is missing the copyright and license header that is present in other test main files in the codebase (such as test/ComponentTestsMain.cpp and demo/main.cpp). Consistent copyright headers should be applied across the codebase.

mhughesacn · 2026-01-26T11:18:44Z

Hi @damcav64 : I have cleared off the code match (i.e. will pss on next commit) but there are a few files that a need a header.
Thank you - MartinH, RDK CMF Compliance Team

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 16 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-27T17:40:01Z

test/cpp_test/cpp/utils.cpp

+    struct curl_slist* headers = nullptr;
+    headers = curl_slist_append(headers, "Content-Type: application/json");
+    curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
+
+    std::string response;
+    curl_easy_setopt(
+        curl, CURLOPT_WRITEFUNCTION,
+        +[](char* ptr, size_t size, size_t nmemb, std::string* data)
+        {
+            data->append(ptr, size * nmemb);
+            return size * nmemb;
+        });
+    curl_easy_setopt(curl, CURLOPT_WRITEDATA, &response);
+
+    CURLcode res = curl_easy_perform(curl);
+    curl_easy_cleanup(curl);


Memory allocated with curl_slist_append for the headers list is never freed. This creates a memory leak. After curl_easy_perform and before curl_easy_cleanup, you should call curl_slist_free_all(headers) to release the header list.

Copilot · 2026-01-27T17:40:01Z

test/cpp_test/cpp/utils.cpp

+    struct curl_slist* headers = nullptr;
+    headers = curl_slist_append(headers, "Content-Type: application/json");
+    curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
+
+    std::string response;
+    curl_easy_setopt(
+        curl, CURLOPT_WRITEFUNCTION,
+        +[](char* ptr, size_t size, size_t nmemb, std::string* data)
+        {
+            data->append(ptr, size * nmemb);
+            return size * nmemb;
+        });
+    curl_easy_setopt(curl, CURLOPT_WRITEDATA, &response);
+
+    CURLcode res = curl_easy_perform(curl);
+    curl_easy_cleanup(curl);


Memory allocated with curl_slist_append for the headers list is never freed. This creates a memory leak. After curl_easy_perform and before curl_easy_cleanup, you should call curl_slist_free_all(headers) to release the header list.

Copilot · 2026-01-27T17:40:01Z

test/cpp_test/cpp/utils.cpp

+    struct curl_slist* headers = nullptr;
+    headers = curl_slist_append(headers, "Content-Type: application/json");


The return value of curl_slist_append is not checked for nullptr. If memory allocation fails, this would result in no headers being set, which could lead to unexpected behavior. Consider checking if curl_slist_append returns nullptr and handling the error appropriately.

Copilot · 2026-01-27T17:40:01Z

test/cpp_test/cpp/outputstream.h

+    OutputStream(const std::string& filename)
+        : file_(std::make_unique<std::ofstream>(filename)),
+          out_(file_.get())
+    {
+    }


The OutputStream constructor doesn't verify if the ofstream was successfully opened. If the file cannot be created or opened (e.g., due to permission issues or invalid path), out_ will point to an invalid stream and subsequent write operations will silently fail. Consider checking file_.is_open() and handling the error appropriately, or at least documenting this behavior.

Copilot · 2026-01-27T17:40:02Z

test/cpp_test/cpp/fireboltdemo.cpp

+    for (const auto& method : json_["methods"])
+    {
+        if (method.contains("name") && method["name"].get<std::string>().rfind(interfaceStr, 0) == 0)
+        {
+
+            names_.push_back(method["name"]); 
+            descriptions_.push_back(method["summary"]); 


Accessing json_["methods"] without checking if the "methods" key exists will throw an exception if it's missing. The same applies to method["name"] and method["summary"]. Consider using the .at() method with exception handling or .value() with a default value to make the code more robust.

Suggested change

for (const auto& method : json_["methods"])

{

if (method.contains("name") && method["name"].get<std::string>().rfind(interfaceStr, 0) == 0)

{

names_.push_back(method["name"]);

descriptions_.push_back(method["summary"]);

// Safely access the "methods" array in the JSON.

auto methodsIt = json_.find("methods");

if (methodsIt == json_.end() || !methodsIt->is_array())

{

return methodNames;

}

for (const auto& method : *methodsIt)

{

// Use .value() with defaults to avoid exceptions if fields are missing or of wrong type.

std::string methodName = method.value("name", "");

if (!methodName.empty() && methodName.rfind(interfaceStr, 0) == 0)

{

names_.push_back(methodName);

descriptions_.push_back(method.value("summary", ""));

Copilot · 2026-01-27T17:40:04Z

test/cpp_test/cpp/utils.cpp

+{
+    nlohmann::json eventMessage;
+    eventMessage["method"] = method;
+    eventMessage["result"] = nlohmann::json::parse(params);


The nlohmann::json::parse call can throw an exception if params is malformed JSON, but there's no exception handling. If the params string is invalid, the program will terminate. Consider wrapping this in a try-catch block and handling the error gracefully.

Suggested change

eventMessage["result"] = nlohmann::json::parse(params);

try

{

eventMessage["result"] = nlohmann::json::parse(params);

}

catch (const nlohmann::json::parse_error& e)

{

FAIL() << "Failed to parse JSON params for event '" << method

<< "': " << e.what() << std::endl;

return;

}

Copilot · 2026-01-27T17:40:04Z

test/cpp_test/main.cpp

+
+    if (gAutoRun)
+    {
+        gOutput = OutputStream("firebolt_test_output.txt");


The OutputStream is being re-initialized with the same filename that was already set on line 75. This creates a new file handle and may lead to resource leaks since the previous file handle isn't explicitly closed before reassignment. Consider removing this redundant initialization or restructuring the code to avoid reopening the same file.

Suggested change

gOutput = OutputStream("firebolt_test_output.txt");

Copilot · 2026-01-27T17:40:05Z

.github/workflows/ci.yml

+            bash -c " \
+              set -e \
+              && cd /workspace/build/test/cpp_test \
+              && ./cpp_test \


The cpp_test executable is being run without any arguments, but it appears to require a connection to a WebSocket server at ws://127.0.0.1:9998 (see main.cpp line 90). The test will likely fail because no mock server or actual server is set up in the CI environment. Consider either adding the -auto flag for automated testing, setting up a mock server, or handling connection failures gracefully in CI.

Suggested change

&& ./cpp_test \

&& ./cpp_test -auto \

Copilot · 2026-01-27T17:40:05Z

test/cpp_test/main.cpp

+        }
+    }
+
+    std::string url = "ws://127.0.0.1:9998";


The hardcoded WebSocket URL should be configurable via command-line argument or environment variable. This would make the test more flexible and allow it to run in different environments without code changes.

Copilot · 2026-01-27T17:40:05Z

test/cpp_test/cpp/fireboltdemo.cpp

+
+void IFireboltDemo::loadRpc()
+{
+    json_ = nlohmann::json::parse(JSON_DATA);


The JSON parsing (nlohmann::json::parse) can throw an exception if JSON_DATA is malformed, but there's no exception handling. If the JSON is invalid, the program will terminate. Consider wrapping this in a try-catch block and handling the error gracefully.

Suggested change

json_ = nlohmann::json::parse(JSON_DATA);

try

{

json_ = nlohmann::json::parse(JSON_DATA);

}

catch (const nlohmann::json::parse_error& e)

{

gOutput << "Failed to parse JSON_DATA: " << e.what() << std::endl;

// json_ remains in its previous (default) state

}

catch (const std::exception& e)

{

gOutput << "Unexpected error while parsing JSON_DATA: " << e.what() << std::endl;

}

cpp_test first commit

af19817

Copilot AI review requested due to automatic review settings January 26, 2026 05:54

Copilot started reviewing on behalf of damcav64 January 26, 2026 05:54 View session

Copilot AI reviewed Jan 26, 2026

View reviewed changes

damcav64 added 2 commits January 26, 2026 13:55

CI

b3e59ab

PR comments

fdf50ba

Copilot AI review requested due to automatic review settings January 27, 2026 17:35

Copilot started reviewing on behalf of damcav64 January 27, 2026 17:35 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

		interfaces = {new AccessibilityDemo(), new AdvertisingDemo(), new DeviceDemo(), new DisplayDemo(),
		new LifecycleDemo(), new LocalizationDemo(), new PresentationDemo(), new StatsDemo()};

		struct curl_slist* headers = nullptr;
		headers = curl_slist_append(headers, "Content-Type: application/json");

-    for (const auto& method : json_["methods"])
-    {
-        if (method.contains("name") && method["name"].get<std::string>().rfind(interfaceStr, 0) == 0)
-        {
-            names_.push_back(method["name"]);
-            descriptions_.push_back(method["summary"]);
+    // Safely access the "methods" array in the JSON.
+    auto methodsIt = json_.find("methods");
+    if (methodsIt == json_.end() || !methodsIt->is_array())
+    {
+        return methodNames;
+    }
+    for (const auto& method : *methodsIt)
+    {
+        // Use .value() with defaults to avoid exceptions if fields are missing or of wrong type.
+        std::string methodName = method.value("name", "");
+        if (!methodName.empty() && methodName.rfind(interfaceStr, 0) == 0)
+        {
+            names_.push_back(methodName);
+            descriptions_.push_back(method.value("summary", ""));

-    eventMessage["result"] = nlohmann::json::parse(params);
+    try
+    {
+        eventMessage["result"] = nlohmann::json::parse(params);
+    }
+    catch (const nlohmann::json::parse_error& e)
+    {
+        FAIL() << "Failed to parse JSON params for event '" << method
+               << "': " << e.what() << std::endl;
+        return;
+    }

-    json_ = nlohmann::json::parse(JSON_DATA);
+    try
+    {
+        json_ = nlohmann::json::parse(JSON_DATA);
+    }
+    catch (const nlohmann::json::parse_error& e)
+    {
+        gOutput << "Failed to parse JSON_DATA: " << e.what() << std::endl;
+        // json_ remains in its previous (default) state
+    }
+    catch (const std::exception& e)
+    {
+        gOutput << "Unexpected error while parsing JSON_DATA: " << e.what() << std::endl;
+    }

cpp_test first commit #25

Are you sure you want to change the base?

cpp_test first commit #25

Uh oh!

Conversation

damcav64 commented Jan 26, 2026

Uh oh!

rdkcmf-jenkins commented Jan 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

mhughesacn commented Jan 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants